Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Remove separate default DetectorElementBase file #2617

Merged
merged 5 commits into from
Nov 4, 2023

Conversation

paulgessinger
Copy link
Member

DetectorElementBase.hpp has an ifdef that either includes the preprocessor define or previously included the default base class header. We included that header in a numbr of locations by accident, breaking the pattern. This removes the separate header (and erroneous inlcudes) and puts the default interface into DetectorElementBase.hpp inside the ifdef.

`DetectorElementBase.hpp` has an `ifdef` that either includes the
preprocessor define or previously included the default base class
header. We included that header in a numbr of locations by accident,
breaking the pattern. This removes the separate header (and erroneous
inlcudes) and puts the default interface into `DetectorElementBase.hpp`
inside the `ifdef`.
@paulgessinger paulgessinger added this to the next milestone Nov 2, 2023
@github-actions github-actions bot added Component - Core Affects the Core module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins labels Nov 2, 2023
Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Merging #2617 (6b5c01f) into main (f908ed6) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2617   +/-   ##
=======================================
  Coverage   49.53%   49.53%           
=======================================
  Files         473      473           
  Lines       26763    26763           
  Branches    12338    12338           
=======================================
  Hits        13257    13257           
  Misses       4753     4753           
  Partials     8753     8753           
Files Coverage Δ
Core/include/Acts/Geometry/DetectorElementBase.hpp 100.00% <100.00%> (ø)
Core/src/Geometry/ProtoLayer.cpp 68.08% <ø> (ø)
Core/src/Surfaces/Surface.cpp 40.32% <ø> (ø)

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Copy link
Contributor

@andiwand andiwand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch

@kodiakhq kodiakhq bot merged commit eb12644 into acts-project:main Nov 4, 2023
53 checks passed
@github-actions github-actions bot removed the automerge label Nov 4, 2023
@acts-project-service acts-project-service added the Fails Athena tests This PR causes a failure in the Athena tests label Nov 4, 2023
@paulgessinger paulgessinger modified the milestones: next, v31.0.0 Nov 6, 2023
kodiakhq bot pushed a commit that referenced this pull request Nov 7, 2023
This PR rationalizes the documentation a bit. The main points are

- Remove the documentation of the old standalone examples executables
- Restructure the core documentation to be less sprawling
- Move the figures from a central folder closer to the relevant source files
- Remove the full auto API documentation in favor of a combination of hard-coded documented symbols + an auto detection mechanism to catch when we references symbols somewhere
  - This reduces the time it takes to build the docs drastically
- Enable nitpicky generation, which will warn (and fail) if symbols are references that don't have a target.

- [x] Try to auto-generate API listings only for the elements we explicitly call out by class / func roles.

Blocked by:
- #2616 
- #2617 
- #2624
@paulgessinger paulgessinger removed the Fails Athena tests This PR causes a failure in the Athena tests label Nov 16, 2023
wdconinc added a commit to eic/EICrecon that referenced this pull request Dec 21, 2023
github-merge-queue bot pushed a commit to eic/EICrecon that referenced this pull request Dec 21, 2023
### Briefly, what does this PR introduce?
In acts-project/acts#2617 the detail header will
be removed.

### What kind of change does this PR introduce?
- [x] Bug fix (issue: Acts v31 compatibility)
- [ ] New feature (issue #__)
- [ ] Documentation update
- [ ] Other: __

### Please check if this PR fulfills the following:
- [ ] Tests for the changes have been added
- [ ] Documentation has been added / updated
- [ ] Changes have been communicated to collaborators

### Does this PR introduce breaking changes? What changes might users
need to make to their code?
No.

### Does this PR change default behavior?
No.
wdconinc added a commit to eic/EICrecon that referenced this pull request Dec 23, 2023
### Briefly, what does this PR introduce?
In acts-project/acts#2617 the detail header will
be removed.

### What kind of change does this PR introduce?
- [x] Bug fix (issue: Acts v31 compatibility)
- [ ] New feature (issue #__)
- [ ] Documentation update
- [ ] Other: __

### Please check if this PR fulfills the following:
- [ ] Tests for the changes have been added
- [ ] Documentation has been added / updated
- [ ] Changes have been communicated to collaborators

### Does this PR introduce breaking changes? What changes might users
need to make to their code?
No.

### Does this PR change default behavior?
No.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component - Core Affects the Core module Component - Examples Affects the Examples module Component - Plugins Affects one or more Plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants